Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows support #39

Merged
merged 3 commits into from
Jul 21, 2019
Merged

Windows support #39

merged 3 commits into from
Jul 21, 2019

Conversation

cznrhubarb
Copy link
Contributor

This is for #28.
Based on the hint you had in that issue I was able to figure it out. Windows thinks .netrc is a file extension so the common solution is to use a _netrc file instead. I also had to add the HOME environment variable since it's not normally set on Windows.

Something I had to do to get it to build was to comment out the git imports since it couldn't find the library. Not sure what I was doing wrong there though. I'm a complete amateur at Python.

@jesseshieh
Copy link
Contributor

Thank you! I'll take a closer look hopefully tomorrow.

Copy link
Contributor

@jesseshieh jesseshieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I don't have easy access to a windows machine, but these changes look good and the tests pass. As soon as I'm able to verify them manually on windows, I'll merge and cut a new release.

gigalixir/openers/windows.py Show resolved Hide resolved
gigalixir/routers/windows.py Show resolved Hide resolved
gigalixir/netrc.py Show resolved Hide resolved
gigalixir/netrc.py Show resolved Hide resolved
@cznrhubarb
Copy link
Contributor Author

Thanks for taking the time to code review/merge the change! I'm sure Windows support was low on your priority list so I appreciate it! It'll make my workflow a lot easier. 👍

@jesseshieh jesseshieh merged commit 70db71b into gigalixir:master Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants